Skip to content

Add upstream proxy - #104

Draft
eraow wants to merge 17 commits into
coder:mainfrom
eraow:feature/upstream-proxy
Draft

Add upstream proxy #104
eraow wants to merge 17 commits into
coder:mainfrom
eraow:feature/upstream-proxy

Conversation

@eraow

@eraow eraow commented Jul 13, 2026

Copy link
Copy Markdown

Note: This PR was generated with AI assistance.

Summary

Adds support for routing httpjail's outbound requests through an upstream proxy using the standard HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment variables.

Motivation

I want to run httpjail behind a corporate proxy. Without upstream proxy support, httpjail connects directly to destination hosts, which fails in environments where direct egress is blocked.

What this does

  • Uses HTTP_PROXY for HTTP destinations and HTTPS_PROXY for HTTPS destinations.
  • Uses NO_PROXY to select destinations that are contacted directly, following curl 8.14.1: domains match at a label boundary along with their subdomains, a list of exactly * disables proxying, and IP or CIDR entries apply to destinations written as an IP literal. Deliberate divergences are listed in the docs.
  • Also accepts the lowercase http_proxy, https_proxy and no_proxy variants. The uppercase spelling wins, consistent with the rest of the codebase; curl prefers the lowercase one and this is documented as a divergence.
  • Supports http:// and bare host:port proxy addresses, with Basic authentication through the proxy URL. Reaching the proxy itself over TLS (https://proxy) is not supported and is rejected with an explicit error.
  • Sends HTTPS requests through a CONNECT tunnel and forwards plain HTTP requests in absolute-form.
  • Never sends Proxy-Authorization to a destination that NO_PROXY bypasses, nor to an HTTPS destination, whose request travels inside the tunnel to the origin server.
  • Applies timeouts to proxy setup operations without limiting established long-running connections such as WebSocket or gRPC.
  • Removes the parent's proxy environment (HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, NO_PROXY, both spellings) from jailed processes. Inheriting NO_PROXY in particular let a jailed process reach the named hosts directly, with no rule evaluation at all.
  • Keeps upstream proxy initialization logs at debug level so normal CLI output is unaffected.

Proxy configuration is environment-only; no additional command-line option is introduced. Behavior is unchanged when no proxy environment variable is configured.

Usage

Route HTTPS requests through a corporate proxy:

HTTPS_PROXY=http://proxy.example.com:8080 \
  httpjail --js-file rules.js -- curl https://github.com

Route both HTTP and HTTPS requests through the same proxy:

HTTP_PROXY=http://proxy.example.com:8080 \
HTTPS_PROXY=http://proxy.example.com:8080 \
  httpjail --js-file rules.js -- ./my-app

Proxy credentials are supported:

HTTPS_PROXY=http://user:pass@proxy.example.com:8080 \
  httpjail --js-file rules.js -- curl https://github.com

Reach internal hosts without going through the proxy:

HTTPS_PROXY=http://proxy.example.com:8080 \
NO_PROXY=api.internal.example,10.0.0.0/8 \
  httpjail --js-file rules.js -- ./my-app

Manual verification

Given rules allowing only github.com:

const DOMAINS = [
  "github.com",
];

(function () {
  const h = (r.host || "").toLowerCase();
  for (const d of DOMAINS) {
    if (d[0] === ".") {
      if (h === d.slice(1) || h.endsWith(d)) return true;
    } else if (h === d) {
      return true;
    }
  }
  return false;
})();

Allowed request

$ sudo env -u SUDO_UID -u SUDO_GID \
    HTTPS_PROXY=http://proxy.example.com:8080 \
    target/debug/httpjail -vv \
    --js-file rules.js \
    --request-log jail.log \
    curl -s -o /dev/null -w "%{http_code}\n" https://github.com
...
DEBUG httpjail: Routing httpjail upstream requests through the proxy environment
DEBUG httpjail::proxy: Upstream client initialized to route through the upstream proxy
...
200
...

Blocked request

$ sudo env -u SUDO_UID -u SUDO_GID \
    HTTPS_PROXY=http://proxy.example.com:8080 \
    target/debug/httpjail -vv \
    --js-file rules.js \
    --request-log jail.log \
    curl -s -o /dev/null -w "%{http_code}\n" https://example.com
...
403
...

NO_PROXY bypass

This is a sanitized transcript from a reachable internal service; the proxy and
service host names have been replaced with reserved example domains.

$ HTTPS_PROXY=http://proxy.example.com:8080 \
    NO_PROXY=api.internal.example \
    ./target/debug/httpjail --weak -vv --js "true" -- \
    curl -s -o /dev/null -w "%{http_code}\n" https://api.internal.example
...
DEBUG httpjail::upstream: Bypassing upstream proxy for api.internal.example
...
302

The 302 is our internal service's redirect response. Together with the
debug line, it confirms that httpjail reached the service directly instead of
sending the request to the configured upstream proxy.

The jailed process does not inherit that value:

$ NO_PROXY=api.internal.example ./target/debug/httpjail --weak --js "true" -- \
    env | grep -i no_proxy
NO_PROXY=localhost,127.0.0.1,::1
no_proxy=localhost,127.0.0.1,::1

@eraow
eraow marked this pull request as ready for review July 13, 2026 14:24
Comment thread docs/advanced/upstream-proxy.md Outdated
By default httpjail contacts destination servers directly. When httpjail itself
runs in an environment that has no direct internet access — for example behind a
corporate proxy — you can route httpjail's own outbound requests through an
upstream proxy with `--upstream-proxy` (or the `HTTPJAIL_UPSTREAM_PROXY`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why shouldn't httpjail itself respect the standard HTTP_PROXY variables? I think it's clear on its face it wouldn't pass that down to children (as that would invalidate the whole point of the jail).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. For the first release, I’ll remove --upstream-proxy and HTTPJAIL_UPSTREAM_PROXY and use the standard HTTP_PROXY / HTTPS_PROXY environment variables for httpjail’s own egress.
Adding a dedicated CLI option and httpjail-specific env var creates an extra configuration path before we have a concrete need for it. If we later need an explicit per-invocation override, we can add it in a follow-up change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 701d116.

@eraow eraow Aug 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ammario following up on 701d116, which drops --upstream-proxy / HTTPJAIL_UPSTREAM_PROXY in favor of the standard HTTP_PROXY / HTTPS_PROXY for httpjail's own egress.

One question: did you intend for the dedicated flag and env var to be removed entirely, or kept as an explicit override alongside HTTP_PROXY? I went with full removal, but it's easy to bring back either way. Let me know how this looks.

Remove the upstream proxy CLI option and httpjail-specific environment
variable, and resolve httpjail's own upstream proxy from HTTP_PROXY and
HTTPS_PROXY instead.

Also keep the new upstream proxy initialization logs at debug level so normal
CLI output is not affected.
@eraow
eraow requested a review from ammario July 28, 2026 06:33

Copy link
Copy Markdown
Contributor

I reviewed this against the latest main (f66518b). The core approach—a Hyper connector that preserves streaming, pooling, and unbounded established connections—is reasonable, so I don't think the raw line count alone warrants a rewrite. I do think a few changes are needed before merging:

  1. NO_PROXY / no_proxy is not honored. With HTTP_PROXY or HTTPS_PROXY set, every destination of that scheme is proxied, including internal hosts that a standard corporate proxy environment expects to connect to directly. Please either implement bypass matching or explicitly narrow the documented semantics.
  2. Proxy configuration is silently process-global through HTTPS_CLIENT.get_or_init. If one ProxyServer initializes the client, a later call to the public new_with_upstream_proxies constructor can ignore its proxy argument. Keeping the client on ProxyServer / ProxyContext would make the configuration honest and remove hidden global state.
  3. The most intricate path lacks an end-to-end test: HTTPS destination through an upstream CONNECT proxy, followed by destination TLS and an HTTP response. The existing tests separately cover plain HTTP proxying and the CONNECT exchange.
  4. read_connect_status performs one async read per byte. That avoids swallowing tunnel payload, but is contrary to the project's minimal-latency goal. Please use buffered reads while preserving any bytes read beyond the CONNECT headers.

For complexity cleanup, the manual RunArgs Debug implementation appears to be residue from the removed dedicated proxy option and can return to a derive. Test-only/public helpers such as ProxyConnector::new and UpstreamProxies::all could also be narrowed. If TLS-to-proxy (https://proxy) is not a demonstrated requirement for the first release, deferring it would remove a meaningful portion of the stream-erasure and second-TLS-config machinery while still supporting HTTPS destinations through an ordinary HTTP proxy.

Local verification: formatting and cargo clippy --all-targets -- -D warnings pass; all unit tests pass. The full suite encountered the unchanged parallel fixed-port collision in weak_integration_max_tx_bytes.

— Codex, AI review agent

eraow added 7 commits August 7, 2026 17:37
The manual Debug implementation was needed while RunArgs carried a
dedicated upstream proxy option whose value could contain credentials.
That option was removed in 701d116 in favor of the standard HTTP_PROXY /
HTTPS_PROXY environment variables, so the implementation now enumerates
every field in declaration order and produces output identical to the
derive.
Reaching the upstream proxy itself over TLS (an `https://` proxy URL) was
implemented but never a demonstrated requirement, and it was the only
reason the connector had to erase the stream type behind `Box<dyn>` and
carry a second rustls ClientConfig. HTTPS destinations are unaffected:
they are still tunneled through a plain HTTP proxy with CONNECT, which is
the standard corporate proxy configuration.

`https://` proxy URLs are now rejected with an error naming the
limitation rather than silently treated as plain HTTP, and ProxyStream
holds a concrete TcpStream so established tunnels no longer pay for
dynamic dispatch on every read and write.
The upstream client lived in a process-global OnceLock, so its
configuration was decided by whichever ProxyServer was constructed first.
A later call to new_with_upstream_proxies silently discarded its
upstream_proxies argument, and get_client() could install a native-roots
fallback client that no proxy configuration could ever replace.

UpstreamClient::new now builds a client per server and ProxyContext
carries it to the handlers, so what a caller passes is what its requests
use. The public init_client_with_ca and get_client are gone along with the
static, and proxy_request / proxy_https_request take the client from the
context they already receive.

This also removes the pre-initialization fallback path entirely: a context
cannot exist without a client, so the case the warning covered is now
unrepresentable.
Uri::host() returns an IPv6 literal with the square brackets that URI
syntax requires around it, so `https://[::1]/` yields `[::1]`.
host_port_authority() then saw a colon in the host and bracketed it a
second time, making the connector emit `CONNECT [[::1]]:443` with a
matching Host header. The authority is malformed, so an upstream proxy
rejects it and IPv6 literal HTTPS destinations are unreachable.

d08e61c fixed the same class of problem for the proxy's own address, which
is parsed by the url crate and therefore arrives unbracketed; the
destination side comes from Uri::host() and was left doubled. The existing
test passed the bare "::1" directly and so did not exercise the Uri path.

uri_host() now strips the brackets Uri::host() keeps, leaving
host_port_authority() responsible for adding them back in authority
position. The test builds its host from a Uri the way the connector does.
With HTTP_PROXY or HTTPS_PROXY set, every destination of that scheme was
sent to the upstream proxy, including the internal hosts a corporate proxy
environment expects to be reached directly. Such a proxy refuses those
CONNECTs, so httpjail did not work in the environment the feature exists
for.

NO_PROXY now selects destinations to contact directly, following curl
8.14.1: domain entries match the domain and its subdomains at a label
boundary, one leading and one trailing dot are ignored on both sides, a
list of exactly `*` disables proxying, and IP or CIDR entries apply to
destinations written as an IP literal. Domain entries and address entries
never cross-match, as the destination decides which kind can apply.
Entries are parsed once at startup so the request path only compares.

Deliberate divergences, all documented: whitespace separates entries
instead of truncating the list, `/0` covers its address family, a mistyped
CIDR is a startup error rather than silently ignored, non-byte-aligned
IPv6 prefixes match correctly (curl fixed the same defect in 8.17.0), and
the uppercase spelling keeps precedence for consistency with the other
proxy variables.

Two properties the implementation is shaped around:

Proxy-Authorization is now decided by http_auth_for_uri(), which is the
only path the request builder uses. Attaching the header from the caller
would have leaked the proxy's credentials to any bypassed internal host,
and to the origin server itself for HTTPS destinations, whose request
travels inside the CONNECT tunnel.

from_env() never parses an input that cannot affect the outcome: a
wildcard bypass short-circuits before the proxy URLs are read, and the
bypass list is left unparsed when no proxy is configured. Otherwise a
leftover NO_PROXY in an environment with no proxy at all would newly
refuse to start.

Errors and logs name an entry by position and never echo its text, since
a NO_PROXY value can hold a mistakenly pasted proxy URL with credentials
and redact_proxy_spec does not cover text after a slash.
Weak mode merged the parent's NO_PROXY into the value it set for the jailed
process. Every host that value named was then reached directly by that
process, skipping httpjail and any rule evaluation — the jail's purpose,
silently defeated by an environment variable. The hole predates the
upstream proxy work, but that work is what gives NO_PROXY a reason to be
set, so it turns a latent problem into a likely one.

The parent's NO_PROXY is no longer merged; the jailed process gets only
localhost, 127.0.0.1 and ::1, which exist to keep local traffic from
looping back through httpjail.

ALL_PROXY was never stripped anywhere. It can carry the upstream proxy's
credentials, and a process that prefers it over the scheme-specific
variables would talk to that proxy instead of to httpjail. It is now
removed alongside HTTP_PROXY and HTTPS_PROXY in weak mode, native Linux
jails and Docker.

All of these are handled in both spellings. Removing only the uppercase
one leaves the hole open, and for NO_PROXY the lowercase spelling is the
dangerous one: curl reads it first.

The three call sites now share PARENT_PROXY_ENV_VARS and
remove_parent_proxy_env() so the list cannot drift apart again. The weak
mode test sets both spellings of each variable to different values on the
parent, so a leak through either one fails and names itself; asserting
only on the uppercase spelling would have passed while the lowercase one
leaked.
curl decides how to read a NO_PROXY entry from what the destination is,
not from what the entry looks like. When the destination is a host name
every entry — including a bare address — goes through the domain suffix
match, so `NO_PROXY=127.0.0.1` also covers `foo.127.0.0.1` and
`127.0.0.1.`; the latter is the plausible one, since a trailing dot stops
the destination from parsing as an address.

Classifying by the entry instead sent both of those through the proxy. The
entry now keeps its original text alongside the parsed address and offers
it as a domain candidate, which restores curl's behavior without
reclassifying anything per request. Label boundaries still apply, so
`x127.0.0.1` does not match.

An IPv6 entry carries its text as well and simply never matches on that
path, because a host name cannot contain a colon — the same outcome curl
reaches by comparing the token as a string.
@eraow
eraow marked this pull request as draft August 8, 2026 03:45
eraow added 3 commits August 8, 2026 03:49
read_connect_status awaited one read per byte so that it could stop
exactly at the end of the headers: reading further would have consumed
tunnel data with nowhere to put it. A short "HTTP/1.1 200 Connection
established" answer therefore cost around forty awaits and syscalls,
against the project's minimal-latency goal.

The response is now read in 1 KiB chunks and the bytes that overshoot the
headers are carried on the connection instead of being dropped.
ProxyStream replays them before it touches the socket, so the destination
TLS handshake sees exactly the byte order it would have seen before.

Details worth keeping:

- The header terminator is searched from three bytes before the newly read
  data, so a \r\n\r\n split across two reads is still found.
- The size cap is applied to the header end, not to everything buffered. A
  single read can legitimately return headers within the cap plus tunnel
  data that pushes the total past it, and that is a success.
- Only the headers are decoded as UTF-8. What follows is a TLS ClientHello
  and arbitrary binary.
- BytesMut reports nearly unbounded space, so each read is capped with
  BufMut::limit rather than letting the buffer size the read.
- One timeout still covers the whole exchange rather than each read, or a
  proxy dribbling out a byte at a time would never trip a deadline.
The CONNECT exchange and the plain-HTTP forwarding path each had tests,
but nothing exercised them composed: an HTTPS destination reached by
opening a tunnel through the proxy, running the destination TLS handshake
inside it, and getting a response back. That is the most intricate path in
the feature and it was the one with no coverage.

The test stands up a real TLS origin with a self-signed certificate and a
proxy that answers CONNECT and then splices the two sockets, so the
handshake has to succeed over the tunnel for the request to arrive at all.
It asserts the CONNECT names the destination authority rather than the
proxy's own address, and that the origin receives the expected path.

One timeout covers the request, the body collection and the two task
joins together. Bounding only the request would let a regression that
stalls after the response headers, or one that leaves the tunnel copy
running, hang the suite instead of failing it.

Verified to have teeth: skipping the CONNECT fails in three seconds rather
than hanging on the thirty second upstream setup timeout, and a stall
after the headers trips the same bound.

Marking the tunnel proxied, which the test cannot detect, is not a gap
this test should try to close: hyper-util's absolute_form() falls back to
origin-form for HTTPS URIs on its own, so the flag cannot change what the
origin sees. The comment says so rather than claiming coverage it does not
have.

Everything binds to port 0 and no external network, DNS, sudo or Docker is
involved, so it runs in parallel with the rest of the suite.
ProxyConnector::new and UpstreamProxies::all were public but never reached
from production code: new() only wrapped all(), and all() only existed to
feed new(). Both were left over from before from_specs() became the way a
configuration is built, and they advertised a second construction path
that nothing supports.

new() is gone, with_config() is now pub(crate) since its only caller is
UpstreamClient::new in this crate, and all() is #[cfg(test)] so it does not
appear in a normal build at all. The two upstream proxy tests build their
connector the same way now, through with_config.

No behavior or public documentation changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants